chore(ci): remove Go versions below 1.24 from CI matrix #15
Conversation
WalkthroughCI matrix reduced to Go 1.24.x and 1.25.x; Test step sets CGO_ENABLED=1 and tightens grep usage; test artifact writes use explicit echo statements; Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (push)
participant CI as GitHub Actions
participant Test as test job
participant Docker as docker job
Dev->>CI: push/change
CI->>Test: run tests (CGO_ENABLED=1, tightened grep, emit artifact vars via echo)
alt tests pass
Test-->>CI: success (test-status=passed)
CI->>Docker: start docker job (needs: test)
Docker-->>CI: docker job completes
else tests fail
Test-->>CI: failure (produce failure details)
end
Note right of CI: docker job no longer needs `docker-test` or `dependency-review`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the CI workflow to streamline testing and fix workflow dependencies by testing against newer Go versions and correcting job dependency chains.
- Reduced Go version matrix to test only with Go 1.24.x and 1.25.x
- Fixed the
dockerjob dependency chain to prevent impossible job prerequisites
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kjanat
force-pushed
the
claude/remove-ci-old-versions-011CUszbT9qVnsAnwmiFv4JC
branch
from
0c4c3ef to
43a76e0
Compare
Remove CI test runs for Go 1.21.x, 1.22.x, and 1.23.x as the minimum supported version is 1.24.0 (as defined in go.mod). This change: - Removes outdated Go versions from the test matrix - Aligns CI testing with the minimum supported version - Reduces CI execution time by removing unnecessary test runs - Maintains testing coverage for supported versions (1.24.x, 1.25.x) fix(ci): remove impossible dependencies from docker job The docker job runs on push events to master/develop branches, but it was depending on docker-test and dependency-review jobs which only run on pull_request events. This created an impossible dependency chain that prevented the docker job from ever running. This change: - Removes docker-test and dependency-review from docker job dependencies - Keeps only the test job as a dependency (which runs on both events) - Allows docker build & push to run correctly on push events - Maintains PR-specific checks (docker-test, dependency-review) for PRs chore(tooling): add pre-commit configuration Introduces a `.pre-commit-config.yaml` file to automate code quality checks before commits. This configuration includes standard hooks for file hygiene (e.g., trailing whitespace, end-of-file fixes) and integrates `golangci-lint` to lint and format Go code on staged files. This helps enforce code style and catch issues early in the development process.
kjanat
force-pushed
the
claude/remove-ci-old-versions-011CUszbT9qVnsAnwmiFv4JC
branch
from
43a76e0 to
e7de5d0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)
22-29: Consider clarifying the CI integration story.The commented
golangci-lint-fullhook notes "Use this hook if you use pre-commit in CI," but the PR objectives focus on updating the CI workflow separately. Clarify whether this guidance should be active (enabled by default or per-environment config), documented in a README, or remains as-is for future reference. Given that the PR aligns CI with Go 1.24+, ensure any CI-specific configuration is coordinated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks).pre-commit-config.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL Analysis / Analyze (go)
🔇 Additional comments (3)
.pre-commit-config.yaml (3)
1-10: LGTM — Standard pre-commit hooks well-configured.The basic hooks from pre-commit-hooks v6.0.0 provide good foundational checks for whitespace, file formatting, and commit hygiene.
14-21: golangci-lint v2.6.1 supports Go 1.24 as minimum version—configuration is correct.golangci-lint v2.6.1 binaries are built with Go 1.24 and support Go 1.24 as minimum version. The hook configuration appropriately uses
--new-from-rev HEAD --fixto lint only modified files with auto-fixes enabled. Therequire_serial: truesetting prevents concurrent access issues, andpass_filenames: falsecorrectly delegates file selection to the--new-from-revflag.Consider whether your team prefers auto-fixing behavior in pre-commit hooks (via
--fix), as this silently modifies files before commit; some teams prefer error-only reporting at this stage and reserve fixes for CI.
30-44: Config file verified; no issues found.The
.golangci.ymlfile exists in the repository root and is properly configured for Go 1.24+. Thego: "1.24"setting is explicitly set, all enabled linters are current (no deprecated ones), and the pre-commit hooks are correctly configured to detect and verify the config file via the regex pattern. The formatter and linter settings align well with modern Go standards. No action required.
Expands the pre-commit configuration with a wider range of hooks to enforce file quality, validation, security, and Git safety checks. The CI pipeline is updated to: - Correct the `golangci-lint` format command to `fmt`. - Enable CGO for test execution to support the race detector. - Improve the robustness of test report parsing scripts. Additionally, this commit includes minor stylistic and formatting cleanups across various project files.
kjanat
force-pushed
the
claude/remove-ci-old-versions-011CUszbT9qVnsAnwmiFv4JC
branch
from
073ab28 to
8d60670
Compare
Adds `actionlint` to the pre-commit configuration to validate GitHub Actions workflows. Significantly expands the `AGENTS.md` file with a comprehensive summary of new features and changes in Go 1.24 and 1.25, along with actionable recommendations for the project. Additionally, normalizes markdown list formatting across various documentation files for consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/CODE_OF_CONDUCT.md (1)
32-32: Minor style suggestion: Strengthen wording on line 32.The static analysis tool flagged line 32 ("Trolling, insulting or derogatory comments, and personal or political attacks") as a candidate for stronger wording. Consider whether a more direct or emphatic phrasing would better align with the tone of the Code of Conduct's enforcement messaging.
AGENTS.md (1)
67-427: Excellent Go 1.24 & 1.25 reference documentation! Consider fixing heading structure.This is a valuable addition that provides comprehensive guidance on new features in Go 1.24 and 1.25, which aligns well with the PR's objective to update the minimum supported Go version to 1.24.0. The content is thorough and includes actionable recommendations for the project.
However, many subsections use bold emphasis (
**Bold Text**) instead of proper markdown headings. This affects document navigation and accessibility.Consider applying this pattern throughout the new section to use proper markdown headings:
-**Generic Type Aliases** +#### Generic Type AliasesThe static analysis tool correctly identified this issue at multiple lines (77, 85, 92, 97, 101, 106, 111, 117, 128, 132, etc.). Using proper heading syntax (
####for level 4 headings) would improve:
- Document structure and table of contents generation
- Accessibility for screen readers
- Markdown rendering consistency across different platforms
.github/workflows/ci.yml (1)
85-88: Improve consistency in grep usage across test result parsing.Lines 86–88 use
--before patterns (e.g.,grep -c -- "--- PASS:"), but line 85 omits it:TOTAL_TESTS=$(grep -c "=== RUN" test-output.log || echo "0")While "===" does not start with a hyphen, adding
--for consistency and robustness is safer:- TOTAL_TESTS=$(grep -c "=== RUN" test-output.log || echo "0") + TOTAL_TESTS=$(grep -c -- "=== RUN" test-output.log || echo "0")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/CODEOWNERS(1 hunks).github/CODE_OF_CONDUCT.md(1 hunks).github/CONTRIBUTING.md(4 hunks).github/PULL_REQUEST_TEMPLATE.md(1 hunks).github/SECURITY.md(1 hunks).github/workflows/autofix.yml(2 hunks).github/workflows/ci.yml(7 hunks).github/workflows/codeql.yml(1 hunks).gitignore(1 hunks).pre-commit-config.yaml(1 hunks)AGENTS.md(2 hunks)DOCKER.md(2 hunks)Dockerfile(1 hunks)Dockerfile.dev(1 hunks)README.md(7 hunks)Taskfile.yml(2 hunks)internal/exporters/html.go(1 hunks)internal/exporters/output.md(1 hunks)scripts/build.ps1(6 hunks)scripts/build.sh(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- scripts/build.ps1
- DOCKER.md
- scripts/build.sh
🚧 Files skipped from review as they are similar to previous changes (7)
- .gitignore
- .pre-commit-config.yaml
- Dockerfile.dev
- internal/exporters/output.md
- .github/workflows/codeql.yml
- .github/workflows/autofix.yml
- .github/CODEOWNERS
🧰 Additional context used
🪛 LanguageTool
.github/PULL_REQUEST_TEMPLATE.md
[style] ~13-~13: Consider using a different verb for a more formal wording.
Context: ... [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaki...
(FIX_RESOLVE)
.github/CODE_OF_CONDUCT.md
[style] ~32-~32: Try using a synonym here to strengthen your wording.
Context: ...ind - Trolling, insulting or derogatory comments, and personal or political attacks - Pu...
(COMMENT_REMARK)
.github/CONTRIBUTING.md
[grammar] ~36-~36: The verb “Create” needs to be in the to-infinitive form.
Context: ...n issues 7. Update documentation if needed 8. Create a pull request with a clear title and...
(MISSING_TO_BEFORE_A_VERB)
.github/SECURITY.md
[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e regular updates about our progress. - We will notify you when the vulnerability ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
AGENTS.md
[uncategorized] ~42-~42: The preposition ‘to’ seems more likely in this position.
Context: ...k`) - Document error handling rationale in defer blocks when ignoring close errors...
(AI_HYDRA_LEO_REPLACE_IN_TO)
[uncategorized] ~147-~147: Loose punctuation mark.
Context: ...yptography Expansion** - crypto/mlkem: ML-KEM-768/1024 post-quantum key exchan...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~165-~165: Loose punctuation mark.
Context: ...ing Improvements** - testing.B.Loop(): Cleaner syntax replacing manual b.N i...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~165-~165: Possible missing comma found.
Context: ...ements** - testing.B.Loop(): Cleaner syntax replacing manual b.N iteration - Prev...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~200-~200: Loose punctuation mark.
Context: ...cations & Removals - runtime.GOROOT(): Deprecated; use system path instead - `...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~232-~232: Loose punctuation mark.
Context: ...mmand Enhancements** - go build -asan: Now defaults to leak detection at progr...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~332-~332: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...r** on Apple M processors - AMD64 fused multiply-add instructions in v3+ mode -
(HYPHENATED_LY_ADVERB_ADJECTIVE)
README.md
[style] ~106-~106: Using many exclamation marks might seem excessive (in this case: 16 exclamation marks for a text that’s 8553 characters long)
Context: ...o, I don't know the version, but I have [
[uncategorized] ~392-~392: Possible missing comma found.
Context: ... with the standard Articulate Rise JSON format which includes: - Course metadata (tit...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~397-~397: Possible missing preposition found.
Context: ...re - Content items with various types - Media references - Quiz/assessment data - Sty...
(AI_HYDRA_LEO_MISSING_OF)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
77-77: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
85-85: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
92-92: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
97-97: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
101-101: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
106-106: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
111-111: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
117-117: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
128-128: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
132-132: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
145-145: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
152-152: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
163-163: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
169-169: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
178-178: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
183-183: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
188-188: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
193-193: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
230-230: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
240-240: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
248-248: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
255-255: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
261-261: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
267-267: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
275-275: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
281-281: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
287-287: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
298-298: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
310-310: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
328-328: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
335-335: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (11)
.github/CODE_OF_CONDUCT.md (1)
20-26: Formatting-only changes; verify alignment with PR objectives.The changes to lines 20–26 and 30–37 appear to be line-wrapping and reflow edits without semantic changes to the Code of Conduct content. However, this file is unrelated to the stated PR objective (removing Go 1.21–1.23 from the CI matrix). Confirm whether these changes are intentional or a side effect of the broader PR.
Also applies to: 30-37
.github/PULL_REQUEST_TEMPLATE.md (1)
13-32: LGTM! Template checkboxes correctly set to unchecked.The checkboxes are appropriately unchecked in this PR template, allowing users to check the relevant boxes when creating a pull request.
Dockerfile (1)
30-30: LGTM! Whitespace cleanup.This is a formatting-only change that removes trailing whitespace with no functional impact.
README.md (1)
23-450: LGTM! Documentation formatting improvements.These changes consist entirely of formatting adjustments (line breaks, list markers, bold/asterisk styling) with no behavioral changes or broken links.
.github/SECURITY.md (1)
16-44: LGTM! Formatting consistency improvements.These changes update list formatting for consistency without altering the security policy content or semantics.
Taskfile.yml (1)
319-437: LGTM! YAML formatting consistency improvement.The changes make dependency specifications more explicit by using single-quoted strings instead of bare scalars. Both syntaxes are valid YAML, but single-quoted strings provide better clarity. The semantic execution order of tasks remains unchanged.
internal/exporters/html.go (1)
18-18: Template file successfully renamed.The verification confirms that
internal/exporters/html_template.gohtmlexists and matches the updated embed directive. The file rename is complete and the build will not fail..github/workflows/ci.yml (4)
40-42: Go version matrix correctly reduced to 1.24.x and 1.25.x.The matrix aligns with the PR objective and the minimum supported Go version declared in go.mod. This reduces CI execution time as intended.
67-68: Verify that CGO_ENABLED=1 is intentional and doesn't alter test behavior.This environment variable enables C bindings, which is a behavioral change not mentioned in the PR objectives. Confirm that this:
- Is necessary for the test suite.
- Does not cause unexpected differences in test execution (e.g., compilation flags, platform-specific behavior).
- Works correctly across both 1.24.x and 1.25.x.
166-196: Verify the package-level coverage aggregation logic.The new logic at lines 166–196 extracts coverage data and aggregates by package using an awk script. Confirm that:
- The temporary file handling (
$temp_coverage) is robust and doesn't cause issues with concurrent runs.- The package path extraction (sed pattern on line 174) correctly normalizes paths for your repository structure.
- The awk averaging logic (lines 185–194) produces accurate aggregated coverage percentages.
- Edge cases (e.g., packages with no coverage, single-file packages) are handled correctly.
Consider adding a safety check to ensure the temp file is created successfully before writing to it.
369-369: Verify that simplifying docker job dependencies to [test] is intentional.The docker job's
needschanged from presumably[golangci, docker-test, dependency-review]to[test]. Confirm that:
- This simplification does not drop validation gates that should be required before pushing images.
- The docker-test job (which runs on PRs) can still validate Docker builds independently for pull requests, and the docker job (which runs on push to specific branches) correctly depends only on the test suite.
- The workflow still maintains proper separation of concerns: tests on all events, Docker push only on specific branches.
|
|
||
| - Go 1.21 or later | ||
| - Git | ||
| - Go 1.21 or later |
There was a problem hiding this comment.
Update minimum Go version to align with go.mod.
The documentation states "Go 1.21 or later" as a prerequisite, but the PR objectives indicate that the minimum supported version is 1.24.0 as defined in go.mod. Please update this to reflect the correct minimum version.
Apply this diff to fix the version requirement:
-- Go 1.21 or later
+- Go 1.24 or later📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Go 1.21 or later | |
| - Go 1.24 or later |
🤖 Prompt for AI Agents
In .github/CONTRIBUTING.md at line 43, the minimum Go version text currently
reads "Go 1.21 or later" but must match go.mod which requires 1.24.0; update
that line to state "Go 1.24.0 or later" (or "Go 1.24 or later" if you prefer
consistent formatting), save the file, and commit the change so documentation
and go.mod are aligned.
Remove CI test runs for Go 1.21.x, 1.22.x, and 1.23.x as the minimum supported version is 1.24.0 (as defined in go.mod).
This change:
Summary by CodeRabbit